feat: play audio files from Discord attachments#255
Open
afonsojramos wants to merge 8 commits intomainfrom
Open
feat: play audio files from Discord attachments#255afonsojramos wants to merge 8 commits intomainfrom
afonsojramos wants to merge 8 commits intomainfrom
Conversation
afonsojramos
force-pushed
the
feat/attachments-play
branch
2 times, most recently
from
790677f to
d34f24c
Compare
Implement secure audio file playback from Discord attachments: - Add file validation with 50MB size limit - Validate content types (audio/*, video/mp4, video/webm, video/ogg) - Use tempfile for secure temporary file handling - Implement FileRestartable for seekable file playback - Use ffprobe for metadata extraction Co-authored-by: cycle-five <cycle.five@proton.me>
Add ffprobe, reqwest, and tempfile crates: - ffprobe: extract audio file metadata - reqwest: download attachment files - tempfile: secure temporary file creation Co-authored-by: cycle-five <cycle.five@proton.me>
Add FileTooLarge and UnsupportedFileType error variants with corresponding user-facing messages for attachment validation. Co-authored-by: cycle-five <cycle.five@proton.me>
- Add QueryType::File variant for attachment playback - Update play command to accept optional attachment parameter - Make query parameter optional when attachment is provided - Handle File variant in all play modes (End, Next, Jump, etc.) Co-authored-by: cycle-five <cycle.five@proton.me>
- Replace unwrap() calls with safe fallbacks in queue display - Fix get_footer_info to handle invalid URLs - Fix create_now_playing_embed for tracks without thumbnails - Add QUEUE_NO_TITLE and QUEUE_NO_SRC fallback constants
- Fix unnecessary borrows for generic args in queue.rs and remove.rs - Add clippy::result_large_err allows for existing code patterns - Fix zombie_processes warning in youtube.rs by calling wait() - Use std::io::Error::other() instead of new() with ErrorKind::Other - Fix double_ended_iterator_last warning - Remove unnecessary cast in file size check
Reduces ParrotError from 136 bytes to 56 bytes by boxing SerenityError, eliminating the need for #[allow(clippy::result_large_err)] annotations.
afonsojramos
force-pushed
the
feat/attachments-play
branch
from
a99b38b to
95c2ee4
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
.unwrap()calls with proper error handling to prevent panicsChanges
src/sources/file.rs- Secure file download and playback implementation/playcommand now accepts an optionalattachmentparameterFileTooLargeandUnsupportedFileTypeerror typesSecurity Improvements
The original PR #244 had several security issues that have been addressed:
tempfile::NamedTempFile.unwrap()→ Replaced with proper error handlingTest Plan
/playwith a valid audio attachment (mp3, wav, ogg)/playwith a file exceeding 50MB (should show error)/playwith an unsupported file type (should show error)/playwith both query and attachment (query should take precedence)Credits
Based on work from PR #244 by @cycle-five